Test is passing in the internal CI.

FYI: @e-rk , @nordic-piks

There is nothing to test, it's a sample

I understand this notion (sample is not a test) but IMO it is not entirely correct. Verifying if the sample builds is a test and we see on a daily basis that even just building bring us some value (finds bugs). What's more, many samples do have associated tests with them (harness: console, verifying the output). This is the case here. So it is a test verifying the correct execution of the sample.
There are multiple levels of testing. Targeted unit tests should constitute the majority and be the foundation of testing, but system and acceptance tests also have their place. IMO executing and verifying samples can be a great acceptance level testing. Also, there is quite some business value behind making sure samples are working. This is often the first contact with any software.

Yes, that's the whole point! :) There is no need for Twister to go through each sample and test them against dozen of platforms, this just doesn't scale in terms of project infrastructure.

Agree, but IMO we are looking from the wrong perspective. Twister != upstream CI. Twister is a tool to build and execute zephyr tests. It must support usage outside of the upstream CI. So There is no need for Twister to go through each sample and test them against dozen of platforms is correct from the upstream CI perspective. And hence should be resolved at that level. I.e. modifying the workflows and the rules on how the upstream CI selects the scope. And IMO definitely not by limiting the tool capabilities (by allowing execution on a very limited list of platforms).

If we continue using platform_allow to limit upstream's CI execution we will end up where we were 5 years ago #25104. This was the whole reason why we even came with integration_platforms #26382.
Integration_platforms are used for the CI-limiting task (and modify the CI workflow to e.g. always run samples only on integration_platforms) (as documentation suggests https://docs.zephyrproject.org/latest/develop/test/twister.html#:~:text=integration_platforms%3A%20%3CYML%20list%20of%20platforms/boards%3E) and this sample is already limited to native_sim

@PerMac As you can imagine, I don't necessarily review in details all the PRs that I merge and yes, it looks like this one fell through the cracks (and interestingly one that also very well illustrates the problem at hand, since e.g sample.filesystem.shell now takes 52s vs. 24s to run on my machine).
I'm sorry but I still fail to see how this can be sustainable, are we really going to potentially add dozens and dozens of entries to platform_allow for each sample and expecting this to scale with the also dozens of boards added every month?

This was the whole reason why we even came with integration_platforms #26382.
Integration_platforms are used for the CI-limiting task (and modify the CI workflow to e.g. always run samples only on integration_platforms) (as documentation suggests docs.zephyrproject.org/latest/develop/test/twister.html#:~:text=integration_platforms%3A <YML list of platforms/boards>) and this sample is already limited to native_sim

Right, but we also have weekly Twister runs ... I don't think that the "belt and braces" approach of tending towards building every sample for every board is really sustainable for the project, and while I understand there is "business value" for the vendors to make sure samples work on their hardware, there needs to be a way for this to not impact the project and its infrastructure at large.

@kartben

I'm sorry but I still fail to see how this can be sustainable, are we really going to potentially add dozens and dozens of entries to platform_allow for each sample and expecting this to scale with the also dozens of boards added every month?

This is one of cases where platform_allow is needed due to a need of dedicated overlay.
Look at it from another perspective. Should we block "hello world" to a single platform in platform_allow? Without it, the execution time could increase with each platform added to the project.

building every sample for every board is [not] really sustainable for the project

Definitely! But again, it is not the problem, that someone wants to make a given sample compatible with their platforms. The problem is what we (the CI) decides to build/execute. We recognize the unscalability of the current testing approach. This made us to start working on a proper "Test Strategy for Zephyr". The proposal is that we start prioritizing what we test and select tests in scope so there is limited redundancy. We will also claim that the project is not responsible for validating that every test and sample works on every board.

there is "business value" for the vendors to make sure samples work on their hardware, there needs to be a way for this to not impact the project and its infrastructure at large.

We are on the same page. Vendors can run tests on their setups, provide results, and this could grant the covered platforms some bonus marking in (release) docs that it was verified on-target. It is not Zephyr's responsibility to validate it. An existence of some combination of platform+sample won't automatically mean that such combination will be validated by the project. But without the change in this PR the vendor is not able to validate this sample without doing some hard to maintain workarounds.
I don't think there is much value in the build "all-on-all" plan (current weekly). I believe we will drop it and run some more reasonable scope in place.
FYI early draft of the test strategy https://docs.google.com/document/d/17tO3S-84fVx0e9M8g1cccAbunfKEpIZvi7i9bEAsDCo/edit?tab=t.0

@kartben can you revisit this?

This change in itself is just following what was done for other boards in this sample already and what is needed to make this work, i.e. defining CONFIG_NVS and CONFIG_SETTINGS_NVS for each board is a bit of a problem if we had to add this for each board. So each time someone wants to claims support in this sample, they had to add an overlay with the same configs that are not strictly hardware/platform specific. Bottom line is that this sample probably needs some rework..

Take a look at all the configs:

./socs/esp32s2.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./socs/esp32c6.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./socs/esp32_procpu.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./socs/cc1352p7.conf
CONFIG_NVS=y

./socs/esp32s3_procpu.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./socs/esp32c3.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./socs/esp32c3_usb.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./prj.conf
CONFIG_STDOUT_CONSOLE=y
CONFIG_FLASH=y
CONFIG_FLASH_MAP=y

CONFIG_SETTINGS=y
CONFIG_SETTINGS_RUNTIME=y

./boards/s32z2xxdc2_s32z270_rtu0.conf
# Copyright 2024 NXP
# SPDX-License-Identifier: Apache-2.0

CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/native_sim_native_64.conf
# Enable the LittleFS file system.
CONFIG_FILE_SYSTEM=y
CONFIG_FILE_SYSTEM_LITTLEFS=y
CONFIG_SETTINGS_FILE=y
CONFIG_SETTINGS_FILE_PATH="/ff/settings/run"

./boards/qemu_x86.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/s32z2xxdc2_s32z270_rtu1.conf
# Copyright 2024 NXP
# SPDX-License-Identifier: Apache-2.0

CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/mr_canhubk3.conf
# Copyright 2023 NXP
# SPDX-License-Identifier: Apache-2.0

# Use LittleFS file system as Settings backend
CONFIG_FILE_SYSTEM=y
CONFIG_FILE_SYSTEM_LITTLEFS=y
CONFIG_SETTINGS_FILE=y
CONFIG_SETTINGS_FILE_PATH="/ff/settings/run"

./boards/mimxrt1020_evk.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/nrf54h20dk_nrf54h20_cpuapp.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/nrf54l15dk_nrf54l10_cpuapp.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/nrf54l15dk_nrf54l05_cpuapp.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/native_sim.conf
# Enable the LittleFS file system.
CONFIG_FILE_SYSTEM=y
CONFIG_FILE_SYSTEM_LITTLEFS=y
CONFIG_SETTINGS_FILE=y
CONFIG_SETTINGS_FILE_PATH="/ff/settings/run"

./boards/nrf54l15dk_nrf54l15_cpuapp.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y

./boards/yd_esp32_procpu.conf
CONFIG_HEAP_MEM_POOL_SIZE=256
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_MPU_ALLOW_FLASH_WRITE=y

./boards/nrf52840dk_nrf52840.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_ARM_MPU=n

./boards/nrf52dk_nrf52832.conf
CONFIG_NVS=y
CONFIG_SETTINGS_NVS=y
CONFIG_ARM_MPU=n

The interesting and worrying part is that we are spending this effort on a sample, and the settings/nvs tests are getting no attention and filters are on a different lavel using depends_on: nvs, This board being added here is not covered in some of those tests. depends_on: nvs is not great, however somethng along thos lines could have been used in this sample.....

NVS is not a board specific option and we should not have board overlays enabling this for sample, it should be enabled by the sample itself and we should look for a better filter and see if it would be possible to remove the platform_allow here.

example of how this could be improved #86479

and here another one to demonstrate we are spending too much time on samples and running them on hardware when the real tests for the features are suboptimal at some spots and are not executed on platforms and not getting the attention they need re coverage. The ZMS test was an interesting one, not running on ANYTHING.

#86493

Closing in favour of #86479